Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement CryptoRng for EntropyRng and StdRng #323

Merged
merged 2 commits into from
Mar 22, 2018

Conversation

pitdicker
Copy link
Contributor

Fallen through the cracks...

@dhardy
Copy link
Member

dhardy commented Mar 22, 2018

Another thing which may have fallen through the cracks: should BlockRngCore be exported from rand? Possibly not necessary.

@pitdicker
Copy link
Contributor Author

I don't know. I think BlockRngCore may some day play a role with better SIMD support. But for now it is mostly an implementation detail of RNG's. I would a little before re-exporting it.

@dhardy
Copy link
Member

dhardy commented Mar 22, 2018

The only real reason is that ReseedingRng requires BlockRngCore.

@pitdicker
Copy link
Contributor Author

Good point. Shall I add it to this PR?

@dhardy
Copy link
Member

dhardy commented Mar 22, 2018

Looks ready for merge to me

@pitdicker pitdicker merged commit b2f19d3 into rust-random:master Mar 22, 2018
@pitdicker pitdicker deleted the additional_crypto_rng branch March 22, 2018 12:54
@burdges
Copy link
Contributor

burdges commented Mar 23, 2018

Is the point of EntropyRng that it can use woefully insecure things when nothing secure is available?

@pitdicker
Copy link
Contributor Author

😄 Actually quite the opposite.

It just builds on the idea that OsRng is the best choice of secure external randomness, but sometimes it is not available (and that is not only theoretical). EntropyRng falls back to the next best choice of secure randomness. This prevents user code from panicking, or from using really insecure workarounds like using the clock.

@dhardy
Copy link
Member

dhardy commented Mar 23, 2018

Using the clock directly (like I tried doing before this), that is. JitterRng flat out gives up if it doesn't have enough precision to gather secure entropy.

@burdges
Copy link
Contributor

burdges commented Mar 23, 2018

I think an entropy based CSPRNG needs to know when it has enough randomness and refuse to give out random numbers if it does not. We've no error path so this mean it needs to panic. Is that what happens?

@pitdicker
Copy link
Contributor Author

JitterRng is not a CSPRNG, but an entropy collector. It uses variances in the time the CPU takes to execute instructions, and variations in timings for memory access. It does a self-test first to see if the timer is granular enough, and the variance is enough to use.

@burdges
Copy link
Contributor

burdges commented Mar 23, 2018

Apologizes but I ignored the whole JitterRng things under the impression that ThreadRng cannot be a CSPRNG anyways and hence not CryptoRng. We've some argument that a CSPRNG can be based on JitterRng?

@pitdicker
Copy link
Contributor Author

You can read all about it on the author's page (This is just a Rust implementation of his library) www.chronox.de/jent/doc/CPU-Jitter-NPTRNG.html

@dhardy
Copy link
Member

dhardy commented Mar 23, 2018

A CSPRNG only needs enough entropy for seeding, after that it's independent of the entropy source (unless you want to reseed).

JitterRng supplies entropy securely (given a good CPU timer) but is SLOW, i.e. it blocks while gathering entropy.

@burdges
Copy link
Contributor

burdges commented Mar 24, 2018

I'll read it, thanks! I also finally noticed the panic when entropy sources fail, which makes sense. lol

I'd suggest adding this method, both so that users know how to test if an EntropyRng is ready, and to help test that try_fill_bytes works right with a zero length destination.

impl EntropyRng {
    fn is_ready(&mut self) -> bool {
        self.try_fill_bytes(&mut [0u8; 0]).is_ok()
    }
}

I do not understand why EntropySource::None exists. I'd think EntropyRng::new() should either use OsRng if it works or launch JitterRng sooner, so that JitterRng launches sooner. I'll maybe figure this out form reading about JitterRng though.

@pitdicker
Copy link
Contributor Author

@burdges Thanks for giving it such close attention 😄.

The idea to use None instead of doing a first try in new() came from #235 (comment). This way it is possible to do things like X::from_rng(EntropySource::new()).

EntropyRng has no concept of being ready / not ready. For that OsRng would ideally poll on /dev/random, and JitterRng would have to live in its own thread. So I am not sure it adds much.

A test for fill_bytes with a zero length destinations is something to keep in mind!

pitdicker added a commit that referenced this pull request Apr 4, 2018
Implement CryptoRng for EntropyRng and StdRng
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants